@hoangelos Thanks for the contribution 👍🏻
I have several considerations re this change:

1. I think running terraform test command makes sense only when there are TF Test files in the directory: https://developer.hashicorp.com/terraform/language/tests#syntax — «Each Terraform test lives in a test file. Terraform discovers test files are based on their file extension: .tftest.hcl or .tftest.json». This way we'd also ensure support for TF versions prior to Terraform v1.6.0
2. I'm not sure about having Terragrunt specific hook for the TF tests since test command doesn't belong in Terragrunt and hence gets forwarded by Terragrunt to Terraform: https://terragrunt.gruntwork.io/docs/reference/cli-options/#all-terraform-built-in-commands — «Terragrunt is a thin wrapper for Terraform, so except for a few of the special commands defined in these docs, Terragrunt forwards all other commands to Terraform». Having standalone Terragrunt Tests hook makes only sense if there might be some use cases when Terrafrunt-specific command line options need to be passed to the hook (I'm not Terragrunt user, thus I'm can say for sure if there are any use cases of such kind).

Plus add usage examples to README as it done for other hooks

@hoangelos Thanks for the contribution 👍🏻 I have several considerations re this change:

1. I think running terraform test command makes sense only when there are TF Test files in the directory: https://developer.hashicorp.com/terraform/language/tests#syntax — «Each Terraform test lives in a test file. Terraform discovers test files are based on their file extension: .tftest.hcl or .tftest.json». This way we'd also ensure support for TF versions prior to Terraform v1.6.0

2. I'm not sure about having Terragrunt specific hook for the TF tests since test command doesn't belong in Terragrunt and hence gets forwarded by Terragrunt to Terraform: https://terragrunt.gruntwork.io/docs/reference/cli-options/#all-terraform-built-in-commands — «Terragrunt is a thin wrapper for Terraform, so except for a few of the special commands defined in these docs, Terragrunt forwards all other commands to Terraform». Having standalone Terragrunt Tests hook makes only sense if there might be some use cases when Terrafrunt-specific command line options need to be passed to the hook (I'm not Terragrunt user, thus I'm can say for sure if there are any use cases of such kind).

@hoangelos Thanks for the contribution 👍🏻 I have several considerations re this change:

1. I think running terraform test command makes sense only when there are TF Test files in the directory: https://developer.hashicorp.com/terraform/language/tests#syntax — «Each Terraform test lives in a test file. Terraform discovers test files are based on their file extension: .tftest.hcl or .tftest.json». This way we'd also ensure support for TF versions prior to Terraform v1.6.0

terraform test looks for.tftest.hcl or .tftest.json_». in the test directory, which by default is tests. I do agree that it's likely we don't want to do this per changed file, but per repo.

2. I'm not sure about having Terragrunt specific hook for the TF tests since test command doesn't belong in Terragrunt and hence gets forwarded by Terragrunt to Terraform: https://terragrunt.gruntwork.io/docs/reference/cli-options/#all-terraform-built-in-commands — «Terragrunt is a thin wrapper for Terraform, so except for a few of the special commands defined in these docs, Terragrunt forwards all other commands to Terraform». Having standalone Terragrunt Tests hook makes only sense if there might be some use cases when Terrafrunt-specific command line options need to be passed to the hook (I'm not Terragrunt user, thus I'm can say for sure if there are any use cases of such kind).

So, your ignorance of Terragrunt has already created a gap in the hooks. Wrapper is a correct, but not fully accurate. As a wrapper, it doesn't "otters things" before it passes things along to terraform. So for example, you can "generate" files that get created the same for different environments, like providers or versions or backends. And then you might run a fmt on everything, which would catch the formatting issues in the generated files too.... However, terra grunt fmt hook only does hclfmt, which doesn't actually run any terraform fmt, which leaves these generated files as a gap. That's a long way to say, technically most of the terraform commands will need separate terragrunt commands too.

I removed the terra grunt test, I do see the need, but this issue was about terraform test. And I'm not actually sure if terra grunt supports the test arguments and even how that would work. I'll research it and open another issue for this once my research is complete.

Also, should I add a disclaimer in the terraform_test section of the README that this may affect the commit time. Because if the test suite is long.... I'd think that's a give, but you know what they say about assumptions.

1. I think running terraform test command makes sense only when there are TF Test files in the directory: developer.hashicorp.com/terraform/language/tests#syntax — «Each Terraform test lives in a test file. Terraform discovers test files are based on their file extension: .tftest.hcl or .tftest.json». This way we'd also ensure support for TF versions prior to Terraform v1.6.0

terraform test looks for.tftest.hcl or .tftest.json_». in the test directory, which by default is tests. I do agree that it's likely we don't want to do this per changed file, but per repo.

What I mean is that running terraform test makes only sense if the commit comprised changes to either TF config files or TF test files only when there are TF test files in current directory, rather than there are just "only TF config files changed", as there's no sense running terraform test command if there are no TF test files in current directory, including TF versions less than 1.6.0, which have no test command for Terraform CLI.

2. I'm not sure about having Terragrunt specific hook for the TF tests since test command doesn't belong in Terragrunt and hence gets forwarded by Terragrunt to Terraform: terragrunt.gruntwork.io/docs/reference/cli-options/#all-terraform-built-in-commands — «Terragrunt is a thin wrapper for Terraform, so except for a few of the special commands defined in these docs, Terragrunt forwards all other commands to Terraform». Having standalone Terragrunt Tests hook makes only sense if there might be some use cases when Terrafrunt-specific command line options need to be passed to the hook (I'm not Terragrunt user, thus I'm can say for sure if there are any use cases of such kind).

So, your ignorance of Terragrunt has already created a gap in the hooks.

Please don't try and make unreasonable assumptions.

Wrapper is a correct, but not fully accurate.

Yes, I agree, and this is why I commented on this PR.
It looked like there's no sense to require Terragrunt CLI and to relay test command via Terragrunt CLI if terraform test suffices the task. I might be wrong though as I'm not a TG user (which doesn't imply ignorance though).

However, terra grunt fmt hook only does hclfmt, which doesn't actually run any terraform fmt, which leaves these generated files as a gap. That's a long way to say, technically most of the terraform commands will need separate terragrunt commands too.

The TG hclfmt is not a TF command, whilst TG does relay test command directly to TF since it's TF-specific command as opposite to hclfmt being a TG-specific command, which doesn't belong in TF, and hence this means what you're comparing is not of the same kind with what I was talking about.

Also, should I add a disclaimer in the terraform_test section of the README that this may affect the commit time. Because if the test suite is long.... I'd think that's a give, but you know what they say about assumptions.

Yes, you probably should. It makes sense to let consumers know this hook it compatible with Terraform version >=1.6.0 only and will produce failure for all the folks with Terraform version <1.6.0. Along with that it makes sense to remove this hook from the default pre-commit-terraform configuration to maintain backward compatibility by avoiding failure that requires human interaction to revolve the failure by adjusting default pre-commit-terraform config.
And yes, I'd think that's a give, but you know what they say about OSS breaking backward compatibility w/o argued reason and prior (long prior) notification 😏

Please don't take my PR comments as ignorance or criticism. I do appreciate the contribution. Though what we'd want to achieve is the tool that can be used by many, which implies a need to maintain backward compatibility and reasonability of changes that are not backward compatible.

Simplicity vs flexibility, you know…

So are you suggesting we gate the terraform tests with a check to see if there are files that look like test files in the test directory which defaults to not the current directory but a directory called tests.

I do want to point out that terraform test doesn’t fail if it’s run in a directory that doesn’t have tests.

So are you suggesting we gate the terraform tests with a check to see if there are files that look like test files in the test directory which defaults to not the current directory but a directory called tests.

I'm suggesting to run terraform test command only in those directories that either have "TF config files changed AND there are TF test files in the same directory" or have "TF test files changed".
There's no "look like test files" from what I see in the documentation. There's pretty definitive explanation of which exact files are considered TF test files to be honest.

I do want to point out that terraform test doesn’t fail if it’s run in a directory that doesn’t have tests.

I was talking more of a TF version since TF test command was officially introduced with version 1.6.0 (and was not guaranteed to be supported prior to 1.6.0) rather than about TF failing or not.
But... but I must admit I didn't know TF supports test command since version 0.15.0.
So what I was concerned about doesn't make sense with TF versions >=0.15.0.
To be honest I'm not quire sure about which most lowest version of Terraform pre-commit-terraform supports officially. Hence it may (or may not) make sense to support earlier versions of Terraform.
@MaxymVlasov @antonbabenko What do you folks think?

The TG hclfmt is not a TF command, whilst TG does relay test command directly to TF since it's TF-specific command as opposite to hclfmt being a TG-specific command, which doesn't belong in TF, and hence this means what you're comparing is not of the same kind with what I was talking about.

My comment is wn by actual terragrunt usage problem that arose with your hooks. I didn’t look into the hook so I assumed it did both fmt and hclfmt. I used terragrunt_fmt assuming it would at least alert if there were failures in formatting these generated terraform files generated by terragrunt. It didn’t. My CICD caught it. But there should be a terragrunt fmt thank a separate terragrunt_hclfmt.

My comment is wn by actual terragrunt usage problem that arose with your hooks. I didn’t look into the hook so I assumed it did both fmt and hclfmt. I used terragrunt_fmt assuming it would at least alert if there were failures in formatting these generated terraform files generated by terragrunt. It didn’t. My CICD caught it. But there should be a terragrunt fmt thank a separate terragrunt_hclfmt.

I'm a bit confused with your attitude to be honest 😢 It's an open source project and we're welcoming and we do appreciate any contribution. And I'm very much sorry you haven't gained as much satisfaction with pre-commit-terraform as you expected from it. And I hope you'll become a valuable part of this community by bringing in solution(s) for the use cases you've been hitting and that might be a matter for others.

My comment is wn by actual terragrunt usage problem that arose with your hooks. I didn’t look into the hook so I assumed it did both fmt and hclfmt. I used terragrunt_fmt assuming it would at least alert if there were failures in formatting these generated terraform files generated by terragrunt. It didn’t. My CICD caught it. But there should be a terragrunt fmt thank a separate terragrunt_hclfmt.

I'm a bit confused with your attitude to be honest 😢 It's an open source project and we're welcoming and we do appreciate any contribution. And I'm very much sorry you haven't gained as much satisfaction with pre-commit-terraform as you expected from it. And I hope you'll become a valuable part of this community by bringing in solution(s) for the use cases you've been hitting and that might be a matter for others.

I’m sorry if you are reading in an attitude. I’m just pointing out the philosophy of skipping some terragrunt commands contributes to some side effects that aren’t good. Happy to file a bug and contribute to that.

I do want to point out that terraform test doesn’t fail if it’s run in a directory that doesn’t have tests.

I was talking more of a TF version since TF test command was officially introduced with version 1.6.0 (and was not guaranteed to be supported prior to 1.6.0) rather than about TF failing or not. But... but I must admit I didn't know TF supports test command since version 0.15.0. So what I was concerned about doesn't make sense with TF versions >=0.15.0. To be honest I'm not quire sure about which most lowest version of Terraform pre-commit-terraform supports officially. Hence it may (or may not) make sense to support earlier versions of Terraform. @MaxymVlasov @antonbabenko What do you folks think?

Tests in 1.6.0 have breaking changes compared to tests that were before. So there is no reason to support test before 1.6.0 until someone opens a feature request because of opentofu etc.

Tests in 1.6.0 have breaking changes compared to tests that were before. So there is no reason to support test before 1.6.0 until someone opens a feature request because of opentofu etc.

So, to only have this run again 1.6.0 and above how would you recommend me add that. Is there any other hook that does that?

Tests in 1.6.0 have breaking changes compared to tests that were before. So there is no reason to support test before 1.6.0 until someone opens a feature request because of opentofu etc.

So, to only have this run again 1.6.0 and above how would you recommend me add that. Is there any other hook that does that?

Add info notice to README like:

> [!IMPORTANT]
> That hook supports only Terraform 1.6.0+.
> Reason: there are [significant changes](https://github.com/hashicorp/terraform/releases/tag/v1.6.0) to how tests are written and executed, based on feedback from the experimental phase"

Also, you could add a check to hook like this, but for terraform --version -json | grep -E -o '[0-9]+\.[0-9]+'
and compare it with 1.6

@hoangelos howdy. Do you plan to continue working on this PR? Do you need any help from our side?

Also interested in this one. Any updates or anything we can do to help land this?

@jmreicha feel free to fork @hoangelos branch to implement requested changes in this PR, resolve README conflicts, and send a new PR.
Thank you. https://github.com/antonbabenko/pre-commit-terraform/blob/master/.github/CONTRIBUTING.md

Had a little bit of time to play around with this and got something working. I'd like to get some feedback on how folks imagine this should work before I open a PR for cases that I'm not thinking about.

@jmreicha good question. As I'm currently using TF 1.5.7 with atmos as wrapper, so all these test functional is not affecting me at all, and I wrote exactly 0 tf tests, so my knowledge on this not deep.

From what I know, tests in 1.6+ located in special files/directory inside tf root/child module.
So for me, it will be make sense to have ability to:

1. Run test only for changed modules
2. Run all tests for whole repo by pre-commit run -a without runs duplications (in case if it is able somehow run test recursively from specified path)

Try keep it simple, and if you're thinking about adding some extra functional for some good reason - better write them down as possible options in PR description or as "review thread" before start implementing.
Ideally, I wanna see something like https://github.com/antonbabenko/pre-commit-terraform/blob/master/hooks/terragrunt_fmt.sh

So I have the second use case accounted for currently. I suppose it makes sense to account for both, I’m imagining some extra flag to toggle the second case on/off and default the behavior to first case. Does that sound reasonable?

I’m imagining some extra flag to toggle the second case on/off and default the behavior to first case. Does that sound reasonable?

Actually, that already done for you

You just need to create per_dir_hook_unique_part for 1 and run_hook_on_whole_repo for 2

Presumably the ideal case is to run terraform test once for each directory containing modified / affected files / dirs? There are a couple layouts possible, either having xyz.tftest.hcl (where xyz might or might not line up directly with the filename(s) changed), or a subdirectory, configurable, but defaulting to tests/.

Presumably tests should also run if the tests are changed, but figuring out what directory to run from if only tests are changed might be challenging? This is a weird corner case... for example, you're using the subdirectory approach, and have a test-only change (some external expectation changed, or you're adding a new test with no code changes; the changed file is in the subdirectory, but the place you'd want to run it would be a directory up. So, for example, with this structure:

├── README.md
├── examples
│   └── examples.tf
├── main.tf
├── outputs.tf
├── tests
│   └── main.tftest.hcl
├── variables.tf
└── versions.tf

where tests only is modified, you'd actually want to run terraform test one directory level up, and if both tests and the main terraform code is modified, you'd only want to run it in the base directory of the module.

Long term, hopefully they'll add some kind of way to label unit or plan-based tests / integration or acceptance tests separately so that you could also choose to only run a subset.

main.tf
foo.tftest.hcl

Easy one, can be added as is, without any problems

For

main.tf
test/foo.tftest.hcl

and

main.tf
anyfolder/bar.tftest.hcl

Current common::per_dir_hook could be problematic.
It's not a big deal to read env var (or where non-default test folder path set) and set it by default to test/ if not founded.

So basically, we need to deduplicate <unique_path>/ and <unique_path>/<test_folder_name>/
and always run terraform test on <unique_path>/ (if I rightly understand how it works)

To achieve that we can drop <test_folder_name> here

Note, that all what I wrote above will work only in case if tests ALWAYS located or in same folder where TF code located or in subfolder (no matter how deep).

But if there exist any possibility to specify path to test in parent folder, or in same level-folder like:

module/main.tf
tests/foo.tftest.hcl

module/main.tf
bar.tftest.hcl

then proposal above will not cover it.

I have discovered that there are some quirks with how terraform test behaves. For example, the tests directory must be located within the module that is initialized, so the first example above would be invalid.

@wyardley there is a filter filter option to limit execution to a subset of the tests. The convention I have adopted is to split tests into unit (plan) and integration (apply) which influenced my original implementation.

I’ll take the feedback and hopefully should be able to come up with something workable. Thanks for the input and suggestions.